-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid new stack context for task list items #20377
Conversation
- Avoid creating a new stack context on a task list item, this PR does that by removing the `position: relative` which causes a new stack context to be created on that element. And thereby solving that popups weren't able to utilize their z-index and weren't able to show properly.
- Backport go-gitea#20377 - Avoid creating a new stack context on a task list item, this PR does that by removing the `position: relative` which causes a new stack context to be created on that element. And thereby solving that popups weren't able to utilize their z-index and weren't able to show properly. Before: ![image](https://user-images.githubusercontent.com/25481501/179135078-63805ccc-17bc-4e11-8393-cdb6f1916cdd.png) After: ![image](https://user-images.githubusercontent.com/25481501/179135105-09996426-85da-40d7-8777-46a16a9413fe.png)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't quite understand why they ever had the relative position set
Thanks for testing @wxiaoguang, f15f897 fixes the issue by making the checkbox relative to the parent element. |
The root problem is that the popups are part of the regular DOM structure where unintended interactions with stacking context happen with surrounding elements. The real fix is to "portal" out the popup to be a direct descendant of |
Maybe fomantic can actually do popup portalling. Worth a try adding a $el.popup({
popup: document.getElementById('popup'),
movePopup: false,
preserve: true,
}); |
We already use fomantic's popup functionality. However portalling should work by specifying |
Popper looks interesting, don't have any experience with it. @silverwind would you like to take over this PR(or create a new one) otherwise I can have a look into popper some time soon. |
are things fixed ?!? |
Adding status/wip |
My (draft) proposal is : https://github.com/wxiaoguang/gitea/tree/fix-stack-context (35631a7) , remove the |
Rewrite to popper (or maybe even the newer floating-ui, but it's quite complex to implement) will be more involved. It'd be awesome if we get portalling popups to work with Fomantic, otherwise I guess a workaround like done here is borderline acceptable. |
#20393 is a replacement for this PR which uses portalling, so surrounding CSS won't interfere anymore with these popups. |
By appending the tooltips to document.body, we can avoid any stacking context issues caused by surrounding element's CSS. I wasn't able to get this portalling to work with Fomantic popups, so this uses tippy.js instead of Fomantic popups. We should aim to replace all Fomantic popups with this eventually. Fixes: go-gitea#20377
Closing as #20393 is better solution to this. |
position: relative
which causes a new stack context to be created on that element. And thereby solving that popups weren't able to utilize their z-index and weren't able to show properly.Before:
After: